feat(auth): add NIP-FI canonical assertion verifier and contracts - #6776
feat(auth): add NIP-FI canonical assertion verifier and contracts#6776wpfleger96 wants to merge 10 commits into
Conversation
Introduce the closed, provider-neutral contract layer at the root of the NIP-FI federated-identity dependency graph (Phase A, PR 1). This has no dependencies on later PRs and defines no schema, migration, runtime JWKS fetching, binding resolution, or request/proof binding. The module provides: - A multi-issuer assertion-policy registry keyed by exact `(iss, sub)`, with the two deterministic semantic contract identities (`AssertionPolicyId`, `TransportContractId`) derived by length-prefixed domain-separated hashing so benign JWKS rotation never changes policy lineage. - The single `FederatedAssertionVerifier` (`FI-INV-16`) producing the origin-sealed, provider-neutral `VerifiedAssertion` result. - The privacy-preserving four-class denial contract (`FI-INV-13`) with the byte-exact wire text, HTTP status, body, and headers fixed by the spec. Mined from #1476's `buzz-auth` verifier core and corrected to the settled spec (merged NIP-FI docs, #5946): token-class plus `typ` enforcement, OIDC ID-token denial, the fixed lowercase-hex-only `nostr_pubkey` claim, resource-owner/client-subject exclusivity, and spec-exact time arithmetic. Identity is issuer-qualified throughout; no issuer, audience, or claim name is hardcoded — all are deployment configuration. Co-authored-by: Cea Stapleton Cordasco <261786559+cea@users.noreply.github.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…ct gaps Address pass-1 review of the NIP-FI verifier/contracts layer. Bind each AssertionKeySet to the exact issuer it authenticates and reject a token whose signed iss does not match the supplied snapshot, closing a cross-issuer authentication bypass under caller misuse. Reject duplicate JOSE/claim members with a duplicate-detecting deserializer so parser- differential ambiguity cannot smuggle a different value past the signature gate. Fold a versioned verifier-contract fingerprint and the normative size bounds into assertion_policy_id and version transport_contract_id, so a semantic change moves its owning ID while JWKS rotation does not. Model resource-owner vs client-subject as a closed, mutually-exclusive SubjectClassContract that denies ambiguous/unclassifiable subjects and gates client-subject eligibility on a recorded non-collision posture. Require the access-token-only client_id claim for named-compatibility policies so they are provably exclusive with OIDC ID tokens. Preserve identity bytes exactly (no trimming). Deny current-status freshness in this verifier until the status-witness runtime lands, rather than sealing without the witness. Co-authored-by: Cea Stapleton Cordasco <261786559+cea@users.noreply.github.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The pass-1 fix bound each AssertionKeySet to an issuer and compared that label to the policy at verify time, but verify() still took the snapshot as a caller argument and AssertionKeySet::new is public. A request-path caller could therefore construct issuer B's JWKS labelled issuer A and mint a sealed (A, victim) identity — the cross-issuer key-source confusion was only detected for honest mislabelling, not prevented. Make the key source structural: FederatedAssertionVerifier holds a trusted IssuerKeySource and resolves the snapshot internally by the token's signature-authenticated iss. verify() now takes only the token, so there is no seam through which a caller can supply or relabel key material. A missing snapshot for a registered issuer is an unreadable authoritative dependency (KeySourceUnavailable -> AuthorizationUnavailable), not rejected evidence. A defensive re-check still denies a source that violates its issuer-binding contract. PR 3 supplies a live-refreshing IssuerKeySource additively. Co-authored-by: Cea Stapleton Cordasco <261786559+cea@users.noreply.github.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…r-contracts * origin/main: Qualify canonical relay images for staged delivery (#6781) feat(desktop): persist agent addressing across composer messages (#6714) feat: navigate images across message threads (#6705) Add database pressure observability (#6700) revert fixed mention highlight (#6716) highlight search terms in results and messages (#6702) fix(desktop): make lightbox zoom controls interactive (#6710) Support community deletion in versioned media buckets (#6738) Fix TipTap editor mount race (#6779) feat(buzz-agent): gate LLM tool calls on session/request_permission (#5712) Add staging dev relay image workflow (#6709) Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
The pass-2 fix moved cross-issuer forgeability off the verify() request seam but left it at the authority-construction seam: AssertionKeySet::new and the IssuerKeySource trait were public, so an external buzz_auth consumer could build a source returning issuer B's JWKS labelled as issuer A, pass it to the public FederatedAssertionVerifier::new, and mint a sealed (A, victim) identity. Make the accepted issuer->JWKS authority entirely crate-owned: - IssuerKeySource gains a private sealed supertrait, so no external crate can implement it. - AssertionKeySet::new becomes pub(crate); a test-utils-gated for_test keeps the crate-owned source usable from integration tests. - A crate-owned, test-utils-gated StaticIssuerKeySource replaces the external MapKeySource the tests defined. compile_fail doctests (compiled as external consumers) prove neither the trait nor the constructor can be named from outside the crate. The verifier's regression suite lived outside 'just test-unit' (which runs buzz-auth --lib only), so it ran in no CI job. Add the integration suite (--features test-utils) and the doctests to the unit-test recipe so the cross-issuer regression and the seal are actually gated by CI. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…r-contracts * origin/main: Remove public relay signing key fallback (#6729) docs(nest): make commit attribution policy-neutral (#6707) fix(desktop-messages): preserve inline agent mentions with persistent addressing (#6793) Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
A downstream-selectable Cargo feature is not an access-control boundary: features are unified and any dependent could enable buzz-auth's test-utils to reach AssertionKeySet::for_test and the StaticIssuerKeySource, reconstruct a relabelled issuer->JWKS authority, and mint a sealed (A, victim) identity from issuer B's real key. Remove for_test entirely, gate StaticIssuerKeySource and its sealed IssuerKeySource impl on cfg(test) only, and move the verifier regression suite into an in-crate #[cfg(test)] mod tests so it reaches the pub(crate)/cfg(test) primitives with zero public feature surface. No supported feature set now exposes an authority-relabelling path; the compile_fail doctests remain as a default-feature seal regression. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Moving the constructor's only callers under cfg(test) left the non-test lib build with no caller, so clippy -D warnings rejected it as dead code (Rust Lint, Windows Rust). PR 3's JWKS runtime is the intended non-test consumer; narrowly allow dead_code on this one item until it lands rather than deferring the constructor or widening the lint. expect would misfire because the lint does not trigger under cfg(test). Also repoint a stale test comment at the two real compile-fail doctests. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The dead_code compile error previously masked this: clippy could not reach empty_line_after_doc_comments until the lib built. The /// block above test_jwks is module narrative about the shared source, not that function's API docs, so convert it to // and remove the trailing blank. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested on f8db951d4c256f49ec9ab181c68385f350d25d95:
-
Map the missing current-status witness to
authorization_unavailable.verifyreturnsStatusWitnessUnavailablebecause this phase cannot read the policy's required current-status authority (crates/buzz-auth/src/nip_fi/verifier.rs:276-282), butdenial_classmaps onlyKeySourceUnavailabletoAuthorizationUnavailable; this error falls through toEvidenceRejected/403 (:458-468). The settled rejection contract assigns an unreadable required current dependency toauthorization_unavailable/503 (docs/nips/NIP-FI.md:461-476). Please map this variant accordingly and extendcurrent_status_policy_denies_without_witnessto assert the public class and wire response. -
Canonicalize set-valued assertion semantics before deriving
AssertionPolicyId. Policy construction retains caller order and duplicates (config.rs:182-215,361-445), while ID derivation hashes that representation directly for audiences, algorithms, subject-class values, and named required/forbidden claims (:605-642). The verifier consumes these as membership sets (verifier.rs:284,302,606,620-621), so permutations or duplicates change the ID without changing accepted assertion semantics. That contradictsH(canonical assertion-policy contract)and the rule that semantic changes move the owning ID (docs/nips/NIP-FI.md:220-238). Please sort and deduplicate these fields before storage/hashing (or reject non-canonical input) and add permutation/duplicate equivalence coverage.
Relatedly, scope capability capture should use the same canonical-set discipline: it currently sorts only by claim name, leaving equal-key values in token order and retaining duplicates (verifier.rs:660-674; assertion.rs:182-197). Thus semantically equivalent scope sets can produce unequal normalized results. Please canonicalize by (name, value), deduplicate, and add parity tests.
All GitHub CI checks are green. This review used read-only GitHub metadata and source/diffs only; PR code was not checked out or executed.
…lass Three review findings on the NIP-FI verifier: - StatusWitnessUnavailable is an unreadable required current dependency, so it now maps to AuthorizationUnavailable/503 rather than falling through to EvidenceRejected/403 (NIP-FI.md rejection table). - Audiences, algorithms, subject-class value sets, and NamedCompatibility claim lists are consumed as membership sets. Sort and deduplicate them before storage and AssertionPolicyId derivation so permutations and duplicates no longer move the policy ID (H(canonical assertion-policy contract)). - Scope capture canonicalizes by (name, value) with duplicates removed so semantically equal scope sets seal byte-equal capabilities. The sealed authority boundary is unchanged. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested on 8d0587c5e0fe8c45d351c93fcb6b5a70de4b7207:
-
Do not treat
client_idpresence as proof that a named-compatibility JWT is not an OIDC ID token. The normative contract requires named compatibility to be mutually exclusive with every accepted ID-token class and says ID tokens always deny (docs/nips/NIP-FI.md:203-210). Construction accepts any compatibility policy that merely requires aclient_idclaim (crates/buzz-auth/src/nip_fi/config.rs:431-440), while verification checks only claim-name presence/absence (verifier.rs:623-632). An issuer-produced ID token with generic/absenttyp, matching issuer/audience, and an additionalclient_idcan therefore pass. Require and validate an authenticated discriminator whose accepted values are actually disjoint between access and ID tokens, or remove this compatibility mode; add a signed ID-token-shaped regression containingclient_id. -
Honor JWK
key_opsbefore accepting a verification key. The contract rejects incompatible JWK usage (docs/nips/NIP-FI.md:166-169), butvalidate_jwkchecks only optionaluseandalg(crates/buzz-auth/src/nip_fi/verifier.rs:695-709) and ignores parsedkey_ops. A matching key explicitly restricted tokey_ops:["encrypt"]can still verify an assertion and mint aVerifiedAssertion. Whenkey_opsis present, requireverifyand reject incompatible combinations; add a signed-token regression for a matchingkidwithkey_ops:["encrypt"]expectingInvalidKey/ evidence rejected.
This review used read-only GitHub metadata and exact-head source inspection only; PR code was not checked out or executed.
The closed, provider-neutral contract layer at the root of the NIP-FI federated-identity dependency graph — Phase A, PR 1 of the plan. It has no dependencies on any other PR and defines no database schema, migration, runtime JWKS fetching, binding resolution, enrollment, or request/proof binding; those belong to later PRs.
What this adds
IssuerRegistry,IssuerPolicy) keyed by exactiss. Identity is issuer-qualified(iss, sub)throughout — equalsubunder differentissare distinct identities. No issuer URL, audience, or claim name is hardcoded; all are deployment configuration.AssertionPolicyId = H(canonical assertion-policy contract)andTransportContractId = H(canonical transport contract)— derived by length-prefixed, domain-separated SHA-256 so a semantic change moves exactly its owning ID while benign JWKS rotation never changes policy lineage.FederatedAssertionVerifier,FI-INV-16) producing the origin-sealed, provider-neutralVerifiedAssertionnormalized result. Its constructor is crate-private, so unverified claims cannot be promoted into authority.DenialClass,FI-INV-13) with the byte-exact Nostr text, HTTP status, body,Content-Type, andWWW-Authenticatevalues fixed by the spec's rejection table.Corrections applied to the mined source
Mined from the
buzz-authverifier core in #1476 and corrected to the settled spec (the mergeddocs/nips/NIP-FI*.md, #5946), which settled after #1476 was written:typenforcement (at+jwt,nip-fi+jwt, named-compatibilityJWT).iss,aud, andsubmatch.nostr_pubkeyclaim accepted only as lowercase hex of exactly one 32-byte key; bech32 and other aliases deny.client_idforat+jwt.missing_evidenceandevidence_rejecteddenial classes added; spec-exact time arithmetic (now < exp,iat <= now + skew,now < iat + maximum_assertion_age, equality at expiry is expired).Verification
New behavior tests in
crates/buzz-auth/tests/nip_fi_verifier.rssign real ES256 assertions against a fixed test key and cover: the happy path, exact-wire-text for all four denial classes, deterministic and semantic contract IDs, token-class enforcement including ID-token denial,nostr_pubkeyhex handling, time bounds, and multi-issuer selection (same subject across distinct issuers yields distinct identities and policy IDs).Notes
Co-authored-byattribution is preserved for the mined author. The feat(relay): add relay-verified identity binding #1476buzz-authcommits are authored by Cea Stapleton Cordasco (the salvage map records feat(relay): add relay-verified identity binding #1476 under Franco; the git history on that PR is Cea's — surfacing for the attribution/closure record).jsonwebtoken 10.4.0(aws_lc_rs) as a workspace dependency.Dependencies: none.